Conversation
chahak13
left a comment
There was a problem hiding this comment.
Use black to format the code with line length 88
|
I had a doubt in bingham.tcc file, in the function compute stress can you please tell what is the state_vars. |
|
I haven't implemented state variables yet but I think it's just a dictionary. See |
|
Yes, it's a dict to store variables specific to individual material points |
|
In the particles.py class the init function does not have dvolumetric_strain while it has been computed in later functions of the same class. |
|
Thanks for pointing that out. I added it to the init method. The reason it worked fine was that every time a new value is assigned to it so it was not needed to be carried over by jax (un/)flattening but better to have it there. |
There was a problem hiding this comment.
Can you also add a couple more things?
- Add a test case
- Add docstrings & comments wherever needed to document its working
- Check dimensions of the variables. (I think I saw some operations that might raise an error)
I've also added a few more comments. Let me know if you have questions or need any help!
|
In the test cases of bingham material I am getting a deviation from the answer present in the CB-Geo test case from bingham in case 3 and 4 which are for yielded case, The tau value is coming off by a value of 20-30 I have checked the code multiple times but am unable to find any error, the dvolumetric strain is coming correct. I think the strain rate has some error it is coming [-0.25,-0.375,0.0,-0.625,0,0] can you please see once |
|
@SachinJalan Let me see if I can find something. Can you be a little clearer about where you think the error is stemming from? Trying to pin in the issue - you say |
diffmpm/material.py
Outdated
| Arguments | ||
| --------- |
There was a problem hiding this comment.
| Arguments | |
| --------- | |
| Parameters | |
| ---------- |
chahak13
left a comment
There was a problem hiding this comment.
I think we're close to wrapping this up. Just a few small comments.
tests/test_bingham.py
Outdated
| {"pressure": jnp.zeros((2, 1))}), | ||
| ], | ||
| ) | ||
| def test_compute_stress_two_particles(particles, state_vars, element, target, dt): |
There was a problem hiding this comment.
Same as above. Just declare dt in the test directly.
tests/test_bingham.py
Outdated
| @pytest.fixture | ||
| def state_vars(): | ||
| return {"pressure": jnp.zeros(1)} | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def dt(): | ||
| return 1.0 | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "particles, element, target", | ||
| particles_element_targets, | ||
| ) | ||
| def test_compute_stress(particles, state_vars, element, target, dt): | ||
| particles.update_natural_coords(element) | ||
| if element.constraints: | ||
| element.apply_boundary_constraints() | ||
| particles.compute_strain(element, dt) | ||
| stress = particles.material.compute_stress(None, particles, state_vars) | ||
| assert jnp.allclose(stress, target) |
There was a problem hiding this comment.
| @pytest.fixture | |
| def state_vars(): | |
| return {"pressure": jnp.zeros(1)} | |
| @pytest.fixture | |
| def dt(): | |
| return 1.0 | |
| @pytest.mark.parametrize( | |
| "particles, element, target", | |
| particles_element_targets, | |
| ) | |
| def test_compute_stress(particles, state_vars, element, target, dt): | |
| particles.update_natural_coords(element) | |
| if element.constraints: | |
| element.apply_boundary_constraints() | |
| particles.compute_strain(element, dt) | |
| stress = particles.material.compute_stress(None, particles, state_vars) | |
| assert jnp.allclose(stress, target) | |
| @pytest.mark.parametrize( | |
| "particles, element, target", | |
| particles_element_targets, | |
| ) | |
| def test_compute_stress(particles, element, target): | |
| state_vars = {"pressure": jnp.zeros(1)} | |
| dt = 1 | |
| particles.update_natural_coords(element) | |
| if element.constraints: | |
| element.apply_boundary_constraints() | |
| particles.compute_strain(element, dt) | |
| stress = particles.material.compute_stress(None, particles, state_vars) | |
| assert jnp.allclose(stress, target) |
No need to mark them as fixtures if they're just returning a value for one test. Just use them in the test directly.
diffmpm/material.py
Outdated
| # Compute the stress | ||
| def compute_stress(self, dstrain, particles, state_vars:dict): | ||
| """ | ||
| Computes the stress for the Bingham material |
There was a problem hiding this comment.
| Computes the stress for the Bingham material | |
| Computes the stress for the Bingham material. |
Issue #9